Skip to content

Modernize Preferences dialog + embed Key Bindings (treebook nav, descriptions, modeless) - #6672

Closed
heffneil wants to merge 27 commits into
xLightsSequencer:masterfrom
heffneil:preferences-modernize
Closed

Modernize Preferences dialog + embed Key Bindings (treebook nav, descriptions, modeless)#6672
heffneil wants to merge 27 commits into
xLightsSequencer:masterfrom
heffneil:preferences-modernize

Conversation

@heffneil

@heffneil heffneil commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Modernize the Preferences dialog + fold in Key Bindings

Overhauls the desktop Preferences UI and moves keyboard-shortcut editing into it as a single page.

Preferences

  • Left navigation rebuilt as a macOS-Settings-style treebook with colourful per-page icons; pages are sorted alphabetically.
  • Modeless — the dialog can stay open while you keep working in Layout/Sequencer (Show() instead of ShowModal(); OK applies + runs the post-change work, Cancel/close dismiss; reuse an open instance by window name).
  • Enforced minimum size so pages (e.g. the Random Effects lists) can't be narrowed into clipping — wxTreebook doesn't propagate page minimums.
  • Descriptions — every setting on Other, Backup and Video now has a plain-language description line; descriptions hide alongside any per-platform-hidden control so no orphaned text is left behind.
  • Consistent bold section headers (plain text, not wxStaticBox captions, which macOS renders in a smaller font).
  • Video page — codec / bitrate / hardware-decode split out of "Other" into their own page.
  • AI page font matches the rest of the dialog (it renders in a wxPropertyGrid).
  • Random Effects — two-list shuttle that no longer clips effect names (rebuilds list contents after layout so macOS re-measures the column).

Key Bindings

  • Now a page inside Preferences; the File-menu "Key Bindings" item is removed.
  • Friendly action names, a single Shortcut column, per-binding descriptions, a live filter, a Category filter (Effects / Wheel of Effects, Presets, Apply Settings, Commands), alphabetical Action column, and editing via a popup editor.
  • A note explains that key-bound effects appear on the Wheel of Effects when double-clicking the sequencer grid.

Sequencer

  • New bindable action Copy Layers/SubModels to Models (previously only a right-click row-heading command), dispatched to a new EffectsGrid::CopyModelLayersToModelsForSelection().

iPad parity

Desktop-only Preferences/UI work (src-ui-wx/). The iPad app has its own settings surfaces and no equivalent Preferences dialog; the shared src-core/ behavior is unchanged. No iPad code changes are required — captured as a note here per the parity policy.

Notes

  • Branch was cut on June 28 and is behind current master; may need a rebase before merge.
  • Release-note text is staged locally for manual paste into README.txt (not modified here).

🤖 Generated with Claude Code

heffneil and others added 19 commits June 29, 2026 21:44
…tions, filter, All scope, modeless, popup editor

- Action column shows humanized friendly names (override map for audio
  speeds etc.); single Shortcut column renders the real chord
- All ~127 bindings get richer descriptions, shown in Details + tooltip
  (tooltip also shows the raw enum)
- Zebra striping; Details column stretches to fill on resize
- Live filter (tokenized AND); 'All' scope (default) shows every binding
- Modeless window (reuse-by-name; fixes a wxDynamicCast false-match that
  showed the Tip-of-the-Day dialog)
- Editing via a popup editor (Edit button/double-click/Enter); Edit
  disabled when nothing selected; popup leads with name + description
- Edit applies in place (no property-grid rebuild/flash)

Reference branch for embedding into the modernized Preferences dialog.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…uttle

- Use a left-hand wxTreebook (icon beside label) on all platforms; drop
  the macOS native preferences window and the dead xLightsPreferencesPage
  class + mPreferencesEditor member.
- Replace the stock/duplicated/low-res page icons with crisp, theme-aware
  SVG icons (one distinct icon per page) built via wxBitmapBundle::FromSVG.
- Rebuild the Random Effects panel as a two-list shuttle (used vs not used)
  with move buttons + double-click, instead of a checkbox grid.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Port the standalone Key Bindings dialog into a new KeyBindingsSettingsPanel
(preferences page): the same filterable, scope-scoped bindings list with
friendly action names, real shortcut column, details, zebra striping, and
per-row tooltip, launching the modal KeyBindingPopupEditor to edit a binding.
Edits apply to the live KeyBindingMap immediately and persist via
keyBindings.Save() when Preferences is accepted.

- Register the page in xLightsPreferences.cpp (keyboard icon).
- Remove the File-menu 'Key bindings' item (MenuItem_KeyBindings, its id,
  Connect, handler, modeless logic) from xLightsMain.{cpp,h} and the .wxs;
  the separate Help-menu 'Key Bindings' cheat-sheet is unchanged.
- Delete the now-unused KeyBindingEditDialog.{cpp,h,wxs} and the dead inline
  property-grid editing path; update .cbp/.vcxproj/.filters.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the single-ink monochrome glyphs with macOS-Settings-style icons:
a distinct rounded coloured tile per page with a white glyph. The tile
carries its own background so the icon stays legible in light and dark
mode without per-theme tinting.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move codec, bitrate and hardware video decode/render controls out of the
Other panel into a new VideoSettingsPanel, registered between Colors and
Other. Render/packaging toggles (GPU render, shaders, exclude audio/video)
stay in Other. Rework OtherSettingsPanel off the wxSmith event table.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Show() instead of ShowModal() so Preferences no longer blocks xLights.
OK applies (panels save via validators in TransferDataFromWindow) and runs
the post-change work (toolbar labels, ResizeMainSequencer, low-def-render
reload) that previously sat after ShowModal(); Cancel/close just dismiss.
Reuse an already-open instance by window name instead of stacking a second.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bump both shuttle wxListBox controls from 190 to 260px wide.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The AI (Services) page renders its settings in a wxPropertyGrid, whose
default font is smaller/denser than the native controls on every other
page, so it looked out of place. Apply the standard GUI font to the grid.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Add StylePreferenceSectionHeaders() helper (PrefPanelUtils.h) that re-fonts
  wxStaticBox captions to the standard GUI font (bold), so section titles match
  the control-label size instead of the smaller native box-caption font. Applied
  to Other, Video, Colors, Backup and Sequence File pages.
- Key Bindings page gains a Category dropdown (All / Effects / Presets /
  Apply Settings / Commands) that filters the list by binding kind; Effects
  shows the EFFECT-type (Wheel of Effects) bindings.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
macOS ignores SetFont on a wxStaticBox caption, so the section titles kept
rendering in the smaller native font. Replace the group boxes on the hand-
written Other and Video pages with a bold wxStaticText header (MakePreference-
SectionHeader) plus indented controls, which renders at full label size.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y, add Wheel-of-Effects note

- Other page: add a greyed one-line description under each setting (email,
  controller-upload link, alias behavior, ping interval -> Status column,
  GPU/shaders, batch-render prompt, purge cache, vendor recommendations,
  custom colour picker) and an intro line under each section header.
- Key Bindings: rename the Effects category to 'Effects / Wheel of Effects'
  and add a note that key-bound effects appear on the Wheel of Effects when
  double-clicking the sequencer grid.
- Add MakePreferenceHint() helper.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…escriptions

Convert the Backup page off wxSmith/wxStaticBox to a hand-written vertical
layout: bold text section headers (Backup Directory, Alternative Backup
Directory) matching the Other page, and a greyed description under each
setting. Behaviour (validation, immediate-apply, transfer) unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sort Key Bindings alphabetically

- New keyboard action COPY_MODEL_LAYERS_TO_MODELS (Sequencer scope) mirroring
  the row-heading menu command; dispatched via MainSequencer to a new
  EffectsGrid::CopyModelLayersToModelsForSelection() that resolves the target
  from the current selection. Registered in KeyBindingTypes with a tip and a
  friendly name so it appears on the Key Bindings page, unassigned by default.
- Key Bindings list now shows the Action column sorted alphabetically
  (collect-then-stable_sort) regardless of storage order.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ipping names

- Sort the preferences pages alphabetically by name after registration (AI
  included), so the left nav reads A-Z.
- Give the Random Effects shuttle lists a real 260px MinSize so wxEXPAND can't
  shrink them below the effect-name width inside the narrow page.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… force Random Effects width

- Other page: the Shaders and Ignore-vendor checkboxes are hidden per-platform;
  hide their description labels alongside them (and Layout()) so no orphaned
  description line is left behind.
- Random Effects: set a 620px min on the panel itself so the scrolled page can't
  render it narrow and clip effect names.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… into clipping

wxTreebook doesn't propagate page minimum sizes, so the dialog had no effective
minimum width and could be shrunk until content (Random Effects lists, etc.)
clipped with an ellipsis. Set a minimum size (nav + full-width page) and grow the
initial size to match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t clipped

On macOS a wxListBox populated before it reaches its final width keeps rendering
items truncated to the old column width. Rebuild each list's contents in a
CallAfter once the panel has its real size so the column re-measures.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 16:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Modernizes the desktop Preferences experience by replacing the prior platform-specific preferences UI with a unified treebook-based, modeless dialog, and embeds the Key Bindings editor as a Preferences page. Also adds a new bindable Sequencer action for copying layers/submodels to other models.

Changes:

  • Replaces Preferences navigation with a wxTreebook-based, modeless dialog and adds shared UI helpers (section headers/hints).
  • Moves “Key bindings” from a File menu modal dialog into a new Preferences page, removing the old dialog and wxSmith artifacts.
  • Adds a new key-bindable Sequencer command (COPY_MODEL_LAYERS_TO_MODELS) and implements selection-aware dispatch in EffectsGrid.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
xLights/Xlights.vcxproj.filters Updates VS filter mappings for new Preferences panels; removes old KeyBindingEditDialog entries.
xLights/Xlights.vcxproj Updates Windows project to compile new Preferences panels and stop compiling removed KeyBindingEditDialog.
xLights/xLights.cbp Updates Code::Blocks project units and wxSmith mappings to remove KeyBindingEditDialog and add new panels.
src-ui-wx/xLightsMain.h Removes menu handler and member declarations for the old File ▸ Key bindings entry.
src-ui-wx/xLightsMain.cpp Removes File menu item creation/handler wiring for the old Key bindings dialog.
src-ui-wx/wxsmith/xLightsframe.wxs Removes the wxSmith menu item definition for Key bindings.
src-ui-wx/wxsmith/RandomEffectsSettingsPanel.wxs Removes wxSmith-generated layout now replaced by custom UI in C++.
src-ui-wx/wxsmith/OtherSettingsPanel.wxs Removes wxSmith-generated layout now replaced by custom UI in C++.
src-ui-wx/wxsmith/KeyBindingEditDialog.wxs Deletes the wxSmith definition for the removed KeyBindingEditDialog.
src-ui-wx/sequencer/MainSequencer.cpp Dispatches the new keybinding action to the EffectsGrid implementation.
src-ui-wx/sequencer/EffectsGrid.h Declares CopyModelLayersToModelsForSelection().
src-ui-wx/sequencer/EffectsGrid.cpp Implements selection-aware wrapper around CopyModelEffectsToModels().
src-ui-wx/preferences/xLightsPreferences.cpp Reworks Preferences into a modeless wxTreebook dialog, adds SVG icons, adds pages (Video, Key Bindings), sorts pages.
src-ui-wx/preferences/VideoSettingsPanel.h Adds new Preferences page API for video decode/export settings.
src-ui-wx/preferences/VideoSettingsPanel.cpp Implements the new Video Preferences page UI and persistence into xLightsFrame.
src-ui-wx/preferences/ServicesPanel.cpp Normalizes wxPropertyGrid font to match other Preferences pages.
src-ui-wx/preferences/RandomEffectsSettingsPanel.h Refactors Random Effects page declarations to match new two-list UI.
src-ui-wx/preferences/RandomEffectsSettingsPanel.cpp Replaces checkbox grid with a two-list shuttle UI to avoid clipping and improve usability.
src-ui-wx/preferences/PrefPanelUtils.h Adds helpers for section headers and hint/description lines.
src-ui-wx/preferences/OtherSettingsPanel.h Refactors members/handlers to match rewritten UI and description-line behavior.
src-ui-wx/preferences/OtherSettingsPanel.cpp Rebuilds Other settings page layout with consistent headers and inline descriptions; moves video settings out.
src-ui-wx/preferences/KeyBindingsSettingsPanel.h Adds new Preferences page for keybinding management.
src-ui-wx/preferences/KeyBindingsSettingsPanel.cpp Implements keybinding list/filter UI and modal popup editor; persists bindings on OK.
src-ui-wx/preferences/BackupSettingsPanel.h Refactors declarations away from wxSmith-generated members toward explicit members/handlers.
src-ui-wx/preferences/BackupSettingsPanel.cpp Rebuilds Backup settings page layout with consistent headers and inline descriptions.
src-ui-wx/app-shell/KeyBindings.cpp Registers the new bindable action and expands/updates keybinding tip strings.
src-ui-wx/app-shell/KeyBindingEditDialog.h Deletes the old modal Key Bindings dialog header.
src-ui-wx/app-shell/KeyBindingEditDialog.cpp Deletes the old modal Key Bindings dialog implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +2 to +4
/***************************************************************
* This source files comes from the xLights project
* https://www.xlights.org
Comment on lines +300 to +306
wxString KeyBindingsSettingsPanel::CategoryOf(const std::string& type)
{
if (type == "EFFECT") return "Effects / Wheel of Effects";
if (type == "PRESET") return "Presets";
if (type == "APPLYSETTING") return "Apply Settings";
return "Commands";
}
Comment on lines +329 to +331
const wxString categorySel = Choice_Category->GetStringSelection();
const bool allCategories = categorySel.empty() || categorySel == "All";

Comment on lines +206 to +210
ListCtrl_Bindings = new wxListCtrl(this, wxID_ANY, wxDefaultPosition, wxSize(-1, 360), wxLC_REPORT | wxLC_SINGLE_SEL);
ListCtrl_Bindings->AppendColumn("Action");
ListCtrl_Bindings->AppendColumn("Shortcut", wxLIST_FORMAT_CENTRE);
ListCtrl_Bindings->AppendColumn("Details");
topSizer->Add(ListCtrl_Bindings, 1, wxEXPAND | wxALL, 4);
Comment thread src-ui-wx/app-shell/KeyBindings.cpp Outdated
{ "CANCEL_RENDER", "Cancel any rendering currently in progress." },
{ "TOGGLE_RENDER", "Turn automatic background rendering on or off." },
{ "PRESETS_TOGGLE", "Show or hide the presets panel." },
{ "FOCUS_SEQUENCER", "Force keyboard focus back to the effects grid when shortcuts stop responding (map this to a function key)." }, // This forces focus to the sequencer for situations where keys dont seem to work. It must be mapped to function key
Comment on lines +308 to +314
KBSCOPE EncodeScope(std::string scope)
{
if (scope == "Controller") return KBSCOPE::Setup;
if (scope == "Layout") return KBSCOPE::Layout;
if (scope == "Sequencer") return KBSCOPE::Sequence;
return KBSCOPE::All;
}
heffneil and others added 3 commits July 8, 2026 14:58
…descriptions

Convert both pages off wxSmith to a hand-written layout with a greyed
description under each setting (surfacing what was previously only in tooltips).
Behaviour and immediate-apply unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…riptions

Convert both pages off wxSmith to a hand-written layout with a greyed
description under each setting. Behaviour and immediate-apply unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the Timing Tracks / Effect Grid / Layout Tab wxStaticBox captions with
bold text headers to match the rest of the modernized Preferences pages; colour
button logic is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 19:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 4 comments.

Comment on lines +33 to +35
#include "KeyBindingsSettingsPanel.h"
#include "KeyBindings.h"
#include "effects/EffectManager.h"
Comment on lines +326 to +331
const wxString scopeSel = Choice_Scope->GetStringSelection();
const bool showAll = (scopeSel == "All");
const KBSCOPE scope = EncodeScope(scopeSel);
const wxString categorySel = Choice_Category->GetStringSelection();
const bool allCategories = categorySel.empty() || categorySel == "All";

Comment on lines +302 to +305
if (type == "EFFECT") return "Effects / Wheel of Effects";
if (type == "PRESET") return "Presets";
if (type == "APPLYSETTING") return "Apply Settings";
return "Commands";
Comment on lines +35 to +39
grid->Add(new wxStaticText(this, wxID_ANY, label), 0, wxALL | wxALIGN_CENTER_VERTICAL, 5);
choice = new wxChoice(this, wxID_ANY);
grid->Add(choice, 0, wxALL, 5);
grid->Add(0, 0);
grid->Add(MakePreferenceHint(this, hint), 0, wxLEFT | wxBOTTOM, 5);
@cybercop23

Copy link
Copy Markdown
Collaborator

Please include a video walkthrough of your changes, audio too, especially for something this elaborate.

# Conflicts:
#	src-ui-wx/app-shell/KeyBindingEditDialog.cpp
#	src-ui-wx/preferences/OtherSettingsPanel.cpp

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 4 comments.

Comment on lines +116 to +121
} else {
#ifdef __WXMSW__
frame->SetHardwareVideoRenderer(HardwareVideoRenderChoice->GetSelection());
HardwareVideoRenderChoice->Enable(HardwareVideoDecodingCheckBox->IsChecked());
#endif
}
@@ -0,0 +1,595 @@

/***************************************************************
* This source files comes from the xLights project
Comment thread src-ui-wx/app-shell/KeyBindings.cpp Outdated
{ "CANCEL_RENDER", "Cancel any rendering currently in progress." },
{ "TOGGLE_RENDER", "Turn automatic background rendering on or off." },
{ "PRESETS_TOGGLE", "Show or hide the presets panel." },
{ "FOCUS_SEQUENCER", "Force keyboard focus back to the effects grid when shortcuts stop responding (map this to a function key)." }, // This forces focus to the sequencer for situations where keys dont seem to work. It must be mapped to function key
return "Commands";
}

KBSCOPE EncodeScope(std::string scope)
… builds

macOS's synchronized groups resolved the bare "KeyBindings.h"; the CMake
Linux/Windows builds need the include-root-relative path used elsewhere
(app-shell/KeyBindings.h), matching MainSequencer.h and EffectWheelDialog.h.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 20:40
# Conflicts:
#	src-ui-wx/preferences/OtherSettingsPanel.cpp

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 5 comments.

Comment on lines +300 to +306
wxString KeyBindingsSettingsPanel::CategoryOf(const std::string& type)
{
if (type == "EFFECT") return "Effects / Wheel of Effects";
if (type == "PRESET") return "Presets";
if (type == "APPLYSETTING") return "Apply Settings";
return "Commands";
}
Comment on lines +329 to +331
const wxString categorySel = Choice_Category->GetStringSelection();
const bool allCategories = categorySel.empty() || categorySel == "All";


wxString KeyBindingsSettingsPanel::RenderShortcut(const KeyBinding& b)
{
if (b.GetKey() == WXK_NONE) return "(unassigned)";

void KeyBindingsSettingsPanel::OnButtonAddPresetClick(wxCommandEvent& event)
{
std::string empty;
Comment on lines +69 to +75
#ifdef __LINUX__
HardwareVideoDecodingCheckBox->Hide();
HardwareVideoRenderChoice->Hide();
#endif
#ifdef __WXOSX__
HardwareVideoRenderChoice->Hide();
#endif
Copilot AI review requested due to automatic review settings July 8, 2026 20:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 2 comments.

Comment on lines +173 to 182
// Modeless so Preferences can stay open while you keep working in xLights.
// Reuse an already-open instance rather than stacking a second dialog.
for (wxWindow* w : wxTopLevelWindows) {
if (w->GetName() == "xlPreferencesDialog") {
w->Show();
w->Raise();
w->SetFocus();
return;
}
}
Comment on lines +29 to +33
// Preferences page for viewing and editing keyboard shortcuts. Hosts the same
// filterable, scope-scoped bindings list the old Key Bindings dialog used, and
// launches a modal popup editor for a single binding. Edits apply to the live
// KeyBindingMap immediately; they are persisted (keyBindings.Save()) when the
// preferences dialog is accepted (TransferDataFromWindow).
… cancel)

With immediate-apply (macOS), settings take effect as they're changed, so a
Cancel button that can't actually revert is misleading. Use a single OK/close
button and run the same finalize (final transfer + toolbar labels, row-height
resize, low-def reload) whether the dialog is closed via the button or its
window close box.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 9, 2026 16:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 4 comments.

Comment on lines +308 to +314
KBSCOPE EncodeScope(std::string scope)
{
if (scope == "Controller") return KBSCOPE::Setup;
if (scope == "Layout") return KBSCOPE::Layout;
if (scope == "Sequencer") return KBSCOPE::Sequence;
return KBSCOPE::All;
}
Comment on lines +577 to +581
void KeyBindingsSettingsPanel::OnButtonAddPresetClick(wxCommandEvent& event)
{
std::string empty;
int id = _keyBindings->AddKey(KeyBinding(false, _(""), _(""), false, false, false, false));
LoadList();
return true;
};
dlg->Bind(wxEVT_BUTTON, [dlg, finalize](wxCommandEvent&) { if (finalize()) dlg->Destroy(); }, wxID_OK);
dlg->Bind(wxEVT_CLOSE_WINDOW, [dlg, finalize](wxCloseEvent&) { finalize(); dlg->Destroy(); });
Comment on lines +98 to 102
topSizer->Add(book, 1, wxEXPAND | wxALL, 5);
// No Cancel: changes apply live (immediate-apply on macOS) so there is
// nothing to cancel; the single button just closes the window.
topSizer->Add(CreateStdDialogButtonSizer(wxOK), 0, wxEXPAND | wxLEFT | wxRIGHT | wxBOTTOM, 5);

- Restore OK/Cancel (removing Cancel broke discard-changes on Windows/Linux
  batch-apply; close/Cancel dismiss without applying).
- Key Bindings: filter by Category index not localized label (fixes empty list
  in non-English locales); make EncodeScope static + take wxString; wrap column
  headers and '(unassigned)' in _(); drop unused local.
- Video page: don't mutate the live renderer setting in batch mode; hide the
  'Hardware Video Renderer' label with its (platform-hidden) choice.
- Preferences dialog: skip an IsBeingDeleted() instance when reusing by name.
- Fix header-comment and 'dont' typos.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 9, 2026 16:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 1 comment.

Comment on lines +176 to +178
// Modeless so Preferences can stay open while you keep working in xLights.
// Reuse an already-open instance rather than stacking a second dialog.
// Reuse an already-open instance rather than stacking a second dialog.
@heffneil

heffneil commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Please include a video walkthrough of your changes, audio too, especially for something this elaborate.

My reply:

  1. I will post screenshots
  2. I have demonstrated this at least 2 separate times in the zoom room
  3. this change doesn't affect the day to day operation of xlights from a sequencing / layout perspective
  4. I can not believe the maintainers aren't building PR's before merging and testing themselves.
  5. if the maintainers are going to base what is merges or not based on the wind than as a contributor I need to know that. Maintainers should build, test and expierence the feature. If the feature doesn't apply to them - take it to the public and let them vote on new features if we are unsure. This particular PR impoves look and feel and usability of the preferences, combines key brindings and leaves it open while using Xlights for the benefit of the user. I would guess a very small population of the users even come in to the screen - perhaps because it looks like it was made on Windows 3.1

@heffneil

heffneil commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Also - show me any other PR where this request has been made to demonstrate the feature

@heffneil

heffneil commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author
Screenshot 2026-07-09 at 3 23 26 PM Screenshot 2026-07-09 at 3 23 17 PM Screenshot 2026-07-09 at 3 23 12 PM Screenshot 2026-07-09 at 3 22 51 PM Screenshot 2026-07-09 at 3 22 43 PM Screenshot 2026-07-09 at 3 22 38 PM Screenshot 2026-07-09 at 3 22 33 PM Screenshot 2026-07-09 at 3 22 28 PM

@dkulp

dkulp commented Jul 9, 2026

Copy link
Copy Markdown
Member

IMO.. this looks very out of place on Mac.... The current settings panel matches the settings panel for almost all the apps (Mail, Safari, Terminal, Messages, etc..)

@computergeek1507

Copy link
Copy Markdown
Member

Also - show me any other PR where this request has been made to demonstrate the feature

#6648 has a video

@cybercop23

Copy link
Copy Markdown
Collaborator

#6474 #6486 #6255 #6268
When the change/enhancment is significant enough, do a video explaining it. 10s of pictures doesn't help.

@heffneil

heffneil commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

#6474 #6486 #6255 #6268 When the change/enhancment is significant enough, do a video explaining it. 10s of pictures doesn't help.

these are complex processes. this pr is screens of options. Nothing exciting - hardly controversial

@heffneil

heffneil commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

IMO.. this looks very out of place on Mac.... The current settings panel matches the settings panel for almost all the apps (Mail, Safari, Terminal, Messages, etc..)

system prefernces are this way. Lots of other things in xlights work this way with a left table and the data or content on the right. the other programs you mentioned people just don't use.

@derwin12

Copy link
Copy Markdown
Contributor

Way too many conflicts now at this point .. also likely needs a bit more discussion about the actual implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants